Cardinal Semiotic Class for Tamil ITN - #452
Conversation
Signed-off-by: retheckj <[email protected]>
Signed-off-by: retheckj-star <[email protected]>
Signed-off-by: retheckj-star <[email protected]>
folivoramanh
left a comment
folivoramanh
left a comment
There was a problem hiding this comment.
add init into data/numbers
add sparrowhawk file and run sh test (we need to make sure both pytest and sparrowhawk pass)
add ta to jenkins file
There was a problem hiding this comment.
only add your language, do not modify the code
There was a problem hiding this comment.
remove this file, and we have run_evaluate to run blind test
There was a problem hiding this comment.
if it's a null file, please remove
| graph = ( | ||
| pynutil.delete('integer: "') | ||
| + pynini.closure( | ||
| pynini.union("௦", "௧", "௨", "௩", "௪", "௫", "௬", "௭", "௮", "௯", " ", "இ", "ன்", "ல்"), |
There was a problem hiding this comment.
why we hardcoded in here?
There was a problem hiding this comment.
It was initially hardcoded to restrict the verbalizer to the required Tamil characters. I'll update it to use the existing reusable graph instead of hardcoding the character set.
|
|
||
| graph_digit = pynini.string_file(get_abs_path("data/numbers/digit.tsv")) | ||
|
|
||
| NEMO_TA_DIGIT = pynini.union("௦", "௧", "௨", "௩", "௪", "௫", "௬", "௭", "௮", "௯").optimize() |
There was a problem hiding this comment.
we have numbers/digit.tsv why we need additional NEMO_TA_DIGIT?
There was a problem hiding this comment.
You're right. NEMO_TA_DIGIT is redundant since we already have data/numbers/digit.tsv. I'll remove the additional definition and reuse the existing graph from digit.tsv instead.
| graph_digit = pynini.string_file(get_abs_path("data/numbers/digit.tsv")) | ||
|
|
||
| NEMO_TA_DIGIT = pynini.union("௦", "௧", "௨", "௩", "௪", "௫", "௬", "௭", "௮", "௯").optimize() | ||
| TAMIL_DIGIT = ["௦", "௧", "௨", "௩", "௪", "௫", "௬", "௭", "௮", "௯"] |
There was a problem hiding this comment.
data/numbers/digit.tsv?
There was a problem hiding this comment.
TAMIL_DIGIT was added only for the integer_to_tamil() helper function to map integer digits to Tamil digits. I'll refactor it to avoid duplicating the digit definitions and reuse data/numbers/digit.tsv where appropriate.
| @@ -1,4 +1,4 @@ | |||
| # Copyright (c) 2021, NVIDIA CORPORATION. All rights reserved. | |||
| # Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved. | |||
folivoramanh
left a comment
folivoramanh
left a comment
There was a problem hiding this comment.
please review all and re-request for further review when done refractor, thanks!
There was a problem hiding this comment.
why we need to list out 10 - 99? is there anyway to graph it? (eg. twenty two is twenty + single number), we dont want wfst run by listing all possible number on tsv? if it must be listing all like this, can you explain the logic for me?
There was a problem hiding this comment.
what the different between lakhs join and lakhs?
There was a problem hiding this comment.
why we need a seperate tsv just to add "ற்"?
There was a problem hiding this comment.
separate tsv file just to add "யே" to all value?
There was a problem hiding this comment.
is it number + "யிரம்"?
There was a problem hiding this comment.
add init.py file to data/number and data/ folder (both 2 folders)
| nemo_text_processing.inverse_text_normalization.ta.utils.get_abs_path("data/numbers/hundreds.tsv") | ||
| ).invert() | ||
|
|
||
| graph_hundreds_join = pynini.string_file( |
There was a problem hiding this comment.
do not abuse .invert(), refractor the tsv data
| self.graph_single_digit = graph_digit | ||
|
|
||
| graph_digit_with_zero = ( | ||
| pynini.cross("ஒன்று", "௦௧") |
There was a problem hiding this comment.
do not hardcode on python file, can we move it to tsv file
There was a problem hiding this comment.
too much graph/self.graph set, but do they pay different role? for self.graph, only set if it is reusable on next semiotic class. And also try to optimize the compositionality (cursor/codex/claude might help you)
| def __init__(self): | ||
| super().__init__(name="punctuation", kind="classify") | ||
|
|
||
| s = "!#$%&\'()*+,-./:;<=>?@^_`{|}~" |
There was a problem hiding this comment.
only this list of punctuation use in tamil?
2 participants
What does this PR do ?
This PR is for the Tamil ITN Cardinal Semiotic Class.
Before your PR is "Ready for review"
Pre checks:
git commit -sto sign.pytestor (if your machine does not have GPU)pytest --cpufrom the root folder (given you marked your test cases accordingly@pytest.mark.run_only_on('CPU')).bash tools/text_processing_deployment/export_grammars.sh --MODE=test ...pytestand Sparrowhawk here.__init__.pyfor every folder and subfolder, includingdatafolder which has .TSV files?Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.to all newly added Python files?Copyright 2015 and onwards Google, Inc.. See an example here.try import: ... except: ...) if not already done.PR Type:
If you haven't finished some of the above items you can still open "Draft" PR.